auth0/src/test/java/com/auth0/android/provider/OAuthManagerStateTest.kt (1)

87-109: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Assert the serialized field explicitly in the default-false test.

Line 87 currently validates only round-trip behavior; it can still pass if serializeToJson() stops emitting dPoPEnabled and deserialization falls back to false.

Suggested patch

     val json = state.serializeToJson()
+    Assert.assertTrue(json.contains("\"dPoPEnabled\":false"))

     val deserializedState = OAuthManagerState.deserializeState(json)

     Assert.assertFalse(deserializedState.dPoPEnabled)

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@auth0/src/test/java/com/auth0/android/provider/OAuthManagerStateTest.kt`
around lines 87 - 109, The test currently only round-trips state via
OAuthManagerState.serializeToJson() and deserializeState(), which can hide
missing serialization of dPoPEnabled; update the test to explicitly assert the
serialized JSON contains the dPoPEnabled field set to false (e.g. check the JSON
string includes the "dPoPEnabled":false key/value) before calling
OAuthManagerState.deserializeState(json), referencing serializeToJson() and
OAuthManagerState.deserializeState so the test fails if the serializer stops
emitting dPoPEnabled.

auth0/src/main/java/com/auth0/android/provider/WebAuthProvider.kt (1)

145-167: ⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Inconsistency: AI summary claims buffering was added, but code shows buffering mechanism was removed.

The AI-generated summary states "added buffering for recovered results until callbacks register," but the code shows the RecoveredResult sealed class, pendingRecovered field, and recoveryLock synchronization were all removed (confirmed by the library context summary). Lines 154–157 and 160–163 directly iterate callbacks without checking for emptiness or storing pending results.

---

Critical: Restored OAuth results are silently dropped if callbacks is empty.

When onRestoreInstanceState reconstructs the OAuthManager after process death and the token exchange completes, the inline callback (lines 153–165) directly iterates the callbacks set. If no caller has invoked addCallback yet—common if the host activity/fragment is still initializing—the iteration produces zero deliveries and the Credentials or AuthenticationException result is permanently lost. Users will never receive their authentication outcome after process death.

The past review comment on this file flagged a similar race with recoveryLock/pendingRecovered and was marked addressed in commit 29ce30e, but the PR objectives note that subsequent revert commits removed the recovery mechanism, leaving the current code without any buffering.

🛡️ Proposed fix: restore buffering or enforce callback registration before restore

Option 1 (recommended): Restore the buffering mechanism

Re-introduce a pending-result holder checked in addCallback:

+    private sealed class RecoveredResult {
+        data class Success(val credentials: Credentials) : RecoveredResult()
+        data class Failure(val error: AuthenticationException) : RecoveredResult()
+    }
+    private val recoveryLock = Any()
+    `@Volatile` private var pendingRecovered: RecoveredResult? = null
+
     `@JvmStatic`
     public fun addCallback(callback: Callback<Credentials, AuthenticationException>) {
         callbacks += callback
+        val pending = synchronized(recoveryLock) {
+            pendingRecovered.also { pendingRecovered = null }
+        }
+        when (pending) {
+            is RecoveredResult.Success -> callback.onSuccess(pending.credentials)
+            is RecoveredResult.Failure -> callback.onFailure(pending.error)
+            null -> {}
+        }
     }

                         override fun onSuccess(result: Credentials) {
+                            val snapshot = synchronized(recoveryLock) {
+                                if (callbacks.isEmpty()) {
+                                    pendingRecovered = RecoveredResult.Success(result)
+                                    return
+                                }
+                                callbacks.toList()
+                            }
-                            for (callback in callbacks) {
+                            for (callback in snapshot) {
                                 callback.onSuccess(result)
                             }
                         }

                         override fun onFailure(error: AuthenticationException) {
+                            val snapshot = synchronized(recoveryLock) {
+                                if (callbacks.isEmpty()) {
+                                    pendingRecovered = RecoveredResult.Failure(error)
+                                    return
+                                }
+                                callbacks.toList()
+                            }
-                            for (callback in callbacks) {
+                            for (callback in snapshot) {
                                 callback.onFailure(error)
                             }
                         }

Option 2: Document that callers must register callbacks before calling onRestoreInstanceState

This shifts the burden to AuthenticationActivity and requires architectural changes; not recommended for an internal recovery path.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@auth0/src/main/java/com/auth0/android/provider/WebAuthProvider.kt` around
lines 145 - 167, The restored OAuth flow can drop results if no callbacks are
registered; reinstate the buffering mechanism: add back the RecoveredResult
sealed class (or equivalent), a nullable pendingRecovered field and a
recoveryLock object, then in onRestoreInstanceState where OAuthManager.fromState
is created store the Credentials or AuthenticationException into
pendingRecovered (synchronized on recoveryLock) when callbacks.isEmpty(),
otherwise deliver to callbacks immediately; finally update addCallback to, under
recoveryLock, check pendingRecovered and deliver and clear it after registering
the new callback so late-registered callbacks receive the buffered result.